feat(agent)!: align skill install with the Agent Skills standard#271
feat(agent)!: align skill install with the Agent Skills standard#271powxenv wants to merge 8 commits into
Conversation
Replace the per-agent bespoke install formats with the Agent Skills open standard: one canonical .agents/skills/<skill>/SKILL.md per project, read directly by universal agents (Codex, Cursor, Cline, Gemini CLI, Copilot, …) and via a relative symlink by every other agent. The bundled registry grows from 8 hand-wired targets to the full standard set (72 agent ids). BREAKING CHANGE: agent install no longer writes per-agent bespoke formats. Codex no longer receives a managed section in AGENTS.md and Windsurf no longer receives a size-capped compact body — both now read the canonical .agents/skills/. The install result schema gains a `mode` (canonical | symlink) field and `path` reflects the new model. Skills written in the old format report stale/modified under `agent status` until refreshed (re-run `agent install`, add --force where needed). The legacy aliases (claude, kiro, copilot) and the claude-code default are preserved, so existing commands keep working.
WalkthroughChangesThe PR adopts canonical Agent Skills files under Canonical skills and target registry
Canonical installation and status flow
Setup defaults and command integration
Skill detection and doctor wiring
Documentation and contribution policy
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AgentInstall
participant CanonicalSkills
participant AgentLanding
User->>AgentInstall: agent install --target <id>
AgentInstall->>CanonicalSkills: write or validate canonical SKILL.md
AgentInstall->>AgentLanding: create symlink or copy fallback
AgentLanding-->>User: install action and path
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ This PR is linked to an issue assigned to @powxenv — thanks! The |
…ehavior - Revise CONTRIBUTING.md to describe universal vs symlinked agent targets and requirements for adding new targets - Update DOCUMENTATION.md to clarify agent install command usage and differentiate universal and symlinked agents - Expand README.md with a detailed supported agents table, including canonical ids, aliases, and skills folder locations - Update src/lib/agent-targets.ts with added agents, corrections to skillsDir and universal flags, and alias mappings - Modify unit tests and e2e tests to reflect renamed and updated agent targets, replacing deprecated aliases like gemini-cli with antigravity-cli and windsurf with devin-desktop - Ensure single source of truth for skills at `.agents/skills`, symlink strategy for non-universal agents, and consistency across docs and code
… chore/agentskills-standard
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/agent-targets.test.ts (1)
131-165: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCoverage doesn't include adversarial alias tokens.
Tests confirm
resolveTargetrejects a plain unknown token (line 156-158), but don't cover prototype-chain keys (constructor,__proto__,toString) that expose the bug flagged inagent-targets.ts(resolveTarget, lines 168-172). Once that's fixed, add a case here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/agent-targets.test.ts` around lines 131 - 165, Add adversarial-token coverage to the resolveTarget tests, specifically asserting that constructor, __proto__, and toString are rejected as unknown tokens. Extend the existing “resolveTarget rejects an unknown token” test in the TARGET_ALIASES + resolveTarget suite, preserving the current null result expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DOCUMENTATION.md`:
- Around line 111-118: Update the agent install examples in the documentation to
pass each agent name through the documented --target option instead of as a
positional argument. Preserve the existing install, list, and status examples
unchanged in purpose.
In `@src/commands/agent.ts`:
- Around line 714-728: Update runStatus around the TARGETS and DEFAULT_SKILLS
iteration to emit universal ok rows only for targets explicitly requested by the
user or targets with an installed actionable artifact. Preserve rows for
actionable states, while suppressing ok-only universal results for uninstalled,
unspecified targets.
In `@src/lib/agent-targets.ts`:
- Around line 168-172: Update resolveTarget’s alias lookup to verify raw is an
own key of TARGET_ALIASES before reading its value, returning null for inherited
keys such as constructor, __proto__, and toString. Preserve the existing TARGETS
lookup and canonical alias behavior, and add regression coverage in
agent-targets.test.ts for these tokens to ensure unknown-target validation is
reached instead of a crash.
---
Nitpick comments:
In `@src/lib/agent-targets.test.ts`:
- Around line 131-165: Add adversarial-token coverage to the resolveTarget
tests, specifically asserting that constructor, __proto__, and toString are
rejected as unknown tokens. Extend the existing “resolveTarget rejects an
unknown token” test in the TARGET_ALIASES + resolveTarget suite, preserving the
current null result expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50a2a944-c182-480c-bf90-a91e921b1d6f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.jsontest/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (18)
CONTRIBUTING.mdDOCUMENTATION.mdREADME.mdskills/testsprite-onboard.skill.mdskills/testsprite-verify.codex.mdskills/testsprite-verify.skill.mdsrc/commands/agent.test.tssrc/commands/agent.tssrc/commands/doctor.tssrc/commands/init.test.tssrc/commands/init.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tssrc/lib/skill-nudge.test.tssrc/lib/skill-nudge.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.tstest/e2e/skill-nudge.e2e.test.ts
💤 Files with no reviewable changes (2)
- skills/testsprite-verify.codex.md
- src/commands/doctor.ts
- Detect legacy own-file skill artifacts and AGENTS.md managed sections during install - Back up legacy files and directories as *.bak before migrating or removing them - Convert legacy skill folders (e.g., claude) to symlink landings pointing to canonical skills - Remove managed section from AGENTS.md and back it up during migration - Implement dry-run mode that plans migration without file changes - Refuse plain installs when legacy artifacts block new symlink target paths - Surface a status advisory listing legacy targets and how to migrate them - Introduce findManagedSectionBounds utility to locate legacy sentinel blocks - Update install action aggregation to consider migrated steps as updated - Add comprehensive tests covering migration scenarios, error cases, and idempotency - Extend filesystem mocks with readdir support for migration operations - Update CLI help text for --force to mention legacy artifact migration and backups
- Update resolveTarget to use hasOwnProperty to exclude inherited prototype keys - Add test to verify resolveTarget returns null for inherited keys like constructor, __proto__, toString, hasOwnProperty - Ensure inherited prototype-chain keys are recognized as unknown targets and rejected explicitly
- Removed multiline conditional and combined it into a single line - Maintained logic to check TARGET_ALIASES property existence before return - Improved code readability and compactness without changing behavior
- Change `testsprite agent install` commands to use `--target` flag for specifying agents - Update DOCUMENTATION.md examples to reflect new command syntax - Modify README.md to replace old command usage with the new format - Clarify onboarding instructions for different agents with consistent command style
…ked agents - Update DOCUMENTATION.md to explain that 'agent status' checks each installed skill against CLI version - Clarify that universal agents share one canonical skill file serving all agents - Explain symlinked agents appear only when their own landing exists - Update CLI help text to reflect these details for 'agent status' command - Adjust snapshot test output to match the revised command description and behavior
What does this PR do?
Aligns
agent installwith the Agent Skills open standard. Instead of hand-wiring a few agents with bespoke formats, the CLI now writes one canonical skill per project at.agents/skills/<skill>/SKILL.md(the standard's shared directory), and each agent reads it directly (universal agents) or via a symlink back to it. The bundled registry grows from 8 targets to the full standard set (72 agent ids), so any skills-compatible agent works out of the box.This broadens adoption (works with 72 agents instead of 8), simplifies maintenance (one uniform code path instead of per-agent
wrap()/mode/compact-body machinery), and follows a vendor-neutral standard already adopted by the agents users run.Related issue
Closes #270
Type of change
Checklist
mainbranch.(
feat(...),fix(...),docs(...), …).npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm testpasses and coverage stays at or above the 80% gate.credentials required).
README.md/DOCUMENTATION.mdwhererelevant.
Notes for reviewers
Breaking changes
This restructures the install model, so existing behavior changes for current users:
AGENTS.md; Windsurf no longer receives a size-capped compact body. Both now read the canonical.agents/skills/(codex directly, windsurf via symlink), like every other agent. Thewrap()/ own-file / managed-section machinery is gone in favor of one standardSKILL.md.agent installresults carry a newmode(canonical|symlink) field, andpathnow reflects the canonical-or-symlink model rather than each agent's bespoke location.stale/modifiedunderagent statusuntil refreshed — re-runningagent install(with--forcewhere needed) brings them in line.What's preserved: the
claude/kiro/copilotaliases still resolve, andagent installwith no--targetstill defaults toclaude-code— so the commands existing scripts use keep working; only the on-disk layout and output change.Implementation notes
.agents/skills/<skill>/SKILL.mdis the single source of truth. Universal agents (Codex, Cursor, Cline, Gemini CLI, Copilot, …) read it directly; every other agent gets a relative symlink from its own skills folder back to it — no drift between agents.lstatand refuse to traverse a planted symlink (exit 5), so a malicious symlink can't redirect a write outside--dir.agent.tsis ~92% lines, with the remainder being defensive error handling and real-fswrappers exercised by the e2e suite.Summary by CodeRabbit
agent installto use a shared canonical skills source, with improved list/status output, dry-run planning, and clearer conflict +--forceoverwrite/backup behavior.claude-code, and onboarding/verification skills include clearer metadata.--targetinstall model, supported-agents tables, and status semantics.